Issue #389: Update self message router and feeback #449
Conversation
- update the self router to not use bendpoints - update feedback to switch from self to oblique and vice versa - update management of start/end messages for execution specifications - add some tests for self messages corner cases - add some (weird) refresh method for tests, to get them green on remote build. working fine locally PDE and maven, but not on remote build server - update anchors on executions to manage automatically the sides. Change-Id: Ieb479d5b1109c4f4b4a76c0df5e45b48c73192d2 Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
There was a problem hiding this comment.
This looks really great. The behaviour of the message routing and feed-back in the diagram editor is so much better. The look-and-feel is now more as it should be. Nice work.
I think most of my comments, if not all, are questions (some of which maybe can be answered in the code). But more significantly, there are sixteen test failures and 62 skips. As it seems those don't happen in the Linux environment of the build server, I'll suppose they are Mac-specific, so I can work on fixing them (as I have the Mac).
Also, there's a problem with self-messages on execution specifications. When I drag a self message down along the execution, the execution resizes correctly. However, if I drag it to the top, it breaks:
And then also I get the familiar error dialog for a cycle in the dependency graph.
...org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java
Outdated
Show resolved
Hide resolved
...rg/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java
Outdated
Show resolved
Hide resolved
...e.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/LifelineBodyAnchor.java
Show resolved
Hide resolved
...rc/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java
Outdated
Show resolved
Hide resolved
...e/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageEndpointEditPolicy.java
Show resolved
Hide resolved
...model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java
Outdated
Show resolved
Hide resolved
...n.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java
Show resolved
Hide resolved
...el/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java
Show resolved
Hide resolved
...el/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java
Outdated
Show resolved
Hide resolved
...el/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java
Outdated
Show resolved
Hide resolved
|
Actually, there's also a test failure that is currently hidden by one of the assumption violations, that indicates a real regression. In test
But worse than this is what happens when attempting the same edit but by moving the execution specification directly (not re-targeting the request message):
This really breaks the diagram, including an error dialog with cycle in the dependency graph. Looks like an inconsistency in the creation/ordering of |
|
Hi, @rschnekenbu. I've made pull request #452 targeting your branch that should clear up the assumption violations on Mac platform. It also fixes some that I think were happening on all platforms (I see them in the build log also), one of which exposes now a test failure that identifies an actual regression in the editor's behaviour. |
Fix several new test assumption violations on Mac platform, along with some compiler warnings in files changed for that reason. Note that one of the LifelineSwitchingUITest cases that was being skipped on an assumption violation is now (accurately) showing a regression on all platforms (a message that becomes a self-message now doesn’t have any vertical extent where it does on the master branch). Also fix the definition of some sub-suites of the LifelineSwitchingUITest that were making wrong assertions/assumptions. Those tests now pass instead of skip. Signed-off-by: Christian W. Damus <give.a.damus@gmail.com>
Fix the remaining test assumption violation that occurs on Linux (not on Mac), including on the build server, which happened to mask a test failure that signals an actual regression in the shape of self-messages. Signed-off-by: Christian W. Damus <give.a.damus@gmail.com>
rschnekenbu
force-pushed
the
rschnekenbu/389-messageRouting
branch
3 times, most recently
from
a5b424c
to
d86f19c
Compare
- simple fixes based on PR review - fix transformation messages to self messages when relevant during move of an execution - Add a similar behavior between moving the end of a sync message starting an execution to another lifeline and the move of the execution itself. Change-Id: I19c9b868f016401fa71a9c5f65c82093fe9ddd8b Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
rschnekenbu
force-pushed
the
rschnekenbu/389-messageRouting
branch
from
d86f19c
to
0ca8483
Compare
rschnekenbu
dismissed
cdamus’s stale review
All comments are adressed, some new issues have been also created for some follow up actions
build. working fine locally PDE and maven, but not on remote build
server
Change-Id: Ieb479d5b1109c4f4b4a76c0df5e45b48c73192d2
Signed-off-by: Remi Schnekenburger rschnekenburger@eclipsesource.com